fix: customize container user permissions using PUID and PGID. #9657#9833
fix: customize container user permissions using PUID and PGID. #9657#9833
Conversation
Add support for custom container user permissions via PUID and PGID environment variables. When the container is started as root (--user root), the pgadmin user is reassigned to the requested UID/GID and all initialization runs under that user via su-exec, ensuring files are created with correct ownership from the start. Key changes: - Dockerfile: add su-exec package, add chmod g=u for /run/pgadmin (fixes OpenShift random UID access) - entrypoint.sh: add PUID/PGID validation and privilege dropping before initialization (not after), preserving OpenShift compatibility Three modes supported: - Default (USER 5050): unchanged behavior - Custom UID (--user root -e PUID=N -e PGID=N): drops to target user before any init - OpenShift (random UID, GID 0): passwd fixup + group permissions
WalkthroughThe pull request modifies Docker configuration and the pgAdmin entrypoint script to support non-root container execution via PUID/PGID environment variables. It adds the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/docker/entrypoint.sh (2)
240-240: Consider using parameter expansion instead of sed.Per static analysis hint (SC2001), bash parameter expansion is more efficient and avoids spawning a subprocess:
♻️ Suggested refactor
- PGADMIN_USER_CONFIG_DIR=$(echo "${PGADMIN_DEFAULT_EMAIL}" | sed 's/@/_/g') + PGADMIN_USER_CONFIG_DIR="${PGADMIN_DEFAULT_EMAIL//@/_}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/docker/entrypoint.sh` at line 240, The assignment to PGADMIN_USER_CONFIG_DIR uses sed and spawns a subprocess; change it to use bash parameter expansion to replace all '@' characters in PGADMIN_DEFAULT_EMAIL with underscores (use the parameter expansion pattern that substitutes globally, e.g. the ${VAR//@/_} form) so PGADMIN_USER_CONFIG_DIR is set without invoking sed.
47-52: Consider documenting potential startup delay for large data volumes.The recursive
chown -Ron/var/lib/pgadminwill traverse all files in the volume. For containers with significant stored data (sessions, query history, server groups), this could noticeably delay startup when using custom PUID/PGID.Consider adding a note to the PR documentation or a log message indicating this is expected behavior:
# Fix ownership of runtime directories BEFORE any initialization for dir in /run/pgadmin /var/lib/pgadmin; do if [ -d "$dir" ]; then + echo "Fixing ownership of $dir (may take time for large volumes)..." chown -R "$PUID:$PGID" "$dir" fi done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/docker/entrypoint.sh` around lines 47 - 52, Add a brief startup warning before the recursive ownership fix in entrypoint.sh: before the for-loop that runs chown -R on /run/pgadmin and /var/lib/pgadmin, emit a log message (e.g., printf or echo to stderr) that indicates recursive chown may take significant time on large volumes and includes the PUID/PGID values; optionally, in the same change consider guarding the chown with a quick ownership check (stat/ls -n) to skip chown if owner/group already match to avoid unnecessary delays.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/docker/entrypoint.sh`:
- Line 240: The assignment to PGADMIN_USER_CONFIG_DIR uses sed and spawns a
subprocess; change it to use bash parameter expansion to replace all '@'
characters in PGADMIN_DEFAULT_EMAIL with underscores (use the parameter
expansion pattern that substitutes globally, e.g. the ${VAR//@/_} form) so
PGADMIN_USER_CONFIG_DIR is set without invoking sed.
- Around line 47-52: Add a brief startup warning before the recursive ownership
fix in entrypoint.sh: before the for-loop that runs chown -R on /run/pgadmin and
/var/lib/pgadmin, emit a log message (e.g., printf or echo to stderr) that
indicates recursive chown may take significant time on large volumes and
includes the PUID/PGID values; optionally, in the same change consider guarding
the chown with a quick ownership check (stat/ls -n) to skip chown if owner/group
already match to avoid unnecessary delays.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1990768e-8eff-4a35-a7c5-4eeb2d7f4585
📒 Files selected for processing (2)
Dockerfilepkg/docker/entrypoint.sh
Summary
Re-implements custom container user permissions via
PUIDandPGIDenvironment variables, fixing the issues that caused the original PR #9633 to be reverted (#9690).Fixes #9657
What changed
Dockerfile
su-execpackage for privilege droppingchmod g=u /run/pgadmin— was missing, required for OpenShift random UIDs to write to this directoryentrypoint.sh
PUID/PGIDenvironment variable support with input validation (must be numeric,PUID=0rejected)--user root): reassignspgadminuser to target UID/GID, fixes ownership of runtime directories before any initialization, runs all commands viasu-execchownwhen running as root$SU_EXECto prevent root-owned file creationWhy the original PR was reverted
The original implementation (#9633) had three critical issues:
USER 5050from Dockerfile — container ran as root by default (security regression)run_pgadmin.pyandload-serverscreated root-owned files in/var/lib/pgadmin. Thechown -Rhappened after init, so if anything failed between init and chown, the volume was left root-owned and inaccessibleusermod/chown, rejected by OpenShift'srestricted-v2SCCHow this fix addresses each issue
USER 5050is preserved — default behavior is completely unchangedrun_pgadmin.py,load-servers,set-prefs) run as the target user viasu-exec, so files are created with correct ownership from the start. No post-hocchownfixup needed for init files.Three supported modes
id -u5050SU_EXEC="", all commands run directly as UID 5050.docker run --user root -e PUID=1000 -e PGID=1000 ...0usermods pgadmin user,chowns runtime dirs, then drops to target user viasu-execfor all init + gunicorn.1001320000SU_EXEC="", passwd fixup adds entry for random UID, files accessible via group permissions (chmod g=u+ GID 0). Nochownneeded.OpenShift compatibility details
On OpenShift with
restricted-v2SCC:USER 5050with a random UID from the namespace range (e.g.,1001320000-1001329999)id -ureturns the random UID, so theelsebranch is takenchmod g=uin the Dockerfile, making them group-writable by GID 0/etc/passwdentry for the random UID (needed forwhoami, Python'sos.getlogin(), etc.)chownis needed — file access works through group permissionsrunAsUser: 5050orfsGroup: 5050in the pod spec — let the SCC assign the UIDThe reported error
.spec.securityContext.fsGroup: Invalid value: []int64{5050}: 5050 is not an allowed groupoccurs when the pod spec explicitly setsfsGroup: 5050, which the SCC rejects. The fix is to remove those fields from the pod spec.Usage examples
Test plan
--user 1001320000:0): container starts, pgAdmin accessible--user root -e PUID=1000 -e PGID=1000): container starts, files owned by 1000:1000Summary by CodeRabbit
Release Notes
Bug Fixes
Chores